Source ClickHouse: add custom jdbc params #17031

sashaNeshcheret · 2022-09-22T10:37:22Z

What

Describe what the change is solving
It helps to add screenshots if it affects the frontend.

How

Describe the solution

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

github-actions · 2022-09-22T10:38:08Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse
source-clickhouse-strict-encrypt

sashaNeshcheret · 2022-09-22T10:39:27Z

/test connector=connectors/source-clickhouse

🕑 connectors/source-clickhouse https://github.com/airbytehq/airbyte/actions/runs/3104780947
✅ connectors/source-clickhouse https://github.com/airbytehq/airbyte/actions/runs/3104780947
No Python unittests run

Build Passed

Test summary info:

All Passed

sashaNeshcheret · 2022-09-22T11:05:52Z

/test connector=connectors/source-clickhouse-strict-encrypt

🕑 connectors/source-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3104940006
✅ connectors/source-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3104940006
No Python unittests run

Build Passed

Test summary info:

All Passed

github-actions · 2022-09-22T11:06:10Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse-strict-encrypt
source-clickhouse

…meters

github-actions · 2022-09-22T11:39:33Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse-strict-encrypt
source-clickhouse

VitaliiMaltsev · 2022-09-23T09:55:54Z

Please add test to describe jdbc url with ssl and additional params

github-actions · 2022-09-23T13:03:28Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse
source-clickhouse-strict-encrypt

github-actions · 2022-09-23T14:24:52Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse
source-clickhouse-strict-encrypt

github-actions · 2022-09-23T14:26:32Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse-strict-encrypt
source-clickhouse

github-actions · 2022-09-23T14:28:11Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse
source-clickhouse-strict-encrypt

grishick · 2022-09-23T17:00:37Z

...rce-clickhouse/src/main/java/io/airbyte/integrations/source/clickhouse/ClickHouseSource.java

+      params.add(SSL_MODE);
+    }
+    if (isAdditionalParamsExists) {
+      params.add(config.get(JdbcUtils.JDBC_URL_PARAMS_KEY).asText());


what will happen if the user puts "sslmode=strict" into jdbc_url_params field? Specifically, will the connection continue to work and what the effective SSL mode of the connection will be in this case?

Seems parameters on clickhouse side puts in map and second parameter with the same key will replace first parameter value, so param passed via jdbc_param_url will have higher priority as far as we firstly set sslmode=none implicitly in code. As far as clickhouse source supports ssl just with none mode and no certificates are created on server, if user puts "sslmode=strict", the following error will be shown: "unable to find valid certification path to requested target", as far as user didn't specify any certificate. @grishick

VitaliiMaltsev · 2022-09-23T17:06:59Z

...rce-clickhouse/src/main/java/io/airbyte/integrations/source/clickhouse/ClickHouseSource.java

-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;


please replace star import

VitaliiMaltsev · 2022-09-23T17:08:42Z

...rce-clickhouse/src/main/java/io/airbyte/integrations/source/clickhouse/ClickHouseSource.java

 import java.util.stream.Collectors;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

+import static io.airbyte.db.jdbc.JdbcUtils.AMPERSAND;


seems like this import is not used in this class

VitaliiMaltsev · 2022-09-23T17:09:22Z

...te/integrations/io/airbyte/integration_tests/sources/ClickHouseJdbcSourceAcceptanceTest.java

 import static java.time.temporal.ChronoUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.*;


please replace star import

github-actions · 2022-09-26T17:29:03Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse-strict-encrypt
source-clickhouse

github-actions · 2022-09-26T18:00:05Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse
source-clickhouse-strict-encrypt

sashaNeshcheret · 2022-09-26T18:04:43Z

/test connector=connectors/source-clickhouse-strict-encrypt

🕑 connectors/source-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3129987080
✅ connectors/source-clickhouse-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3129987080
No Python unittests run

Build Passed

Test summary info:

All Passed

sashaNeshcheret · 2022-09-26T18:04:51Z

/test connector=connectors/source-clickhouse

🕑 connectors/source-clickhouse https://github.com/airbytehq/airbyte/actions/runs/3129987989
✅ connectors/source-clickhouse https://github.com/airbytehq/airbyte/actions/runs/3129987989
No Python unittests run

Build Passed

Test summary info:

All Passed

…-clickhouse-custom-parameters

github-actions · 2022-09-27T09:32:05Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-clickhouse
source-clickhouse-strict-encrypt

sashaNeshcheret · 2022-09-27T10:00:32Z

/publish connector=connectors/source-clickhouse

🕑 Publishing the following connectors:
connectors/source-clickhouse
https://github.com/airbytehq/airbyte/actions/runs/3134686299

Connector	Did it publish?	Were definitions generated?
connectors/source-clickhouse	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

sashaNeshcheret · 2022-09-27T10:38:05Z

/publish connector=connectors/source-clickhouse-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-clickhouse-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3134915842

Connector	Did it publish?	Were definitions generated?
connectors/source-clickhouse-strict-encrypt	✅	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

* Source clickhouse: add jdbc params * Source clickhouse: add jdbc params * Source clickhouse: add jdbc params * Source clickhouse: updated tests * Source clickhouse: format * Source clickhouse: format * Source clickhouse: format * Source clickhouse: format * Source clickhouse: bump version * auto-bump connector version [ci skip] Co-authored-by: Octavia Squidington III <[email protected]>

Source clickhouse: add jdbc params

80f25ed

github-actions bot added the area/connectors Connector related issues label Sep 22, 2022

octavia-squidington-iii added connectors/source/clickhouse connectors/source/clickhouse-strict-encrypt labels Sep 22, 2022

Source clickhouse: add jdbc params

35748b6

sashaNeshcheret requested review from VitaliiMaltsev and yurii-bidiuk September 22, 2022 11:38

sashaNeshcheret marked this pull request as ready for review September 22, 2022 11:38

sashaNeshcheret requested a review from a team as a code owner September 22, 2022 11:38

Merge branch 'master' into omneshcheret/source-clickhouse-custom-para…

97e5b41

…meters

sashaNeshcheret linked an issue Sep 22, 2022 that may be closed by this pull request

Source Clickhouse: custom JDBC parameters #10723

Closed

Source clickhouse: add jdbc params

ab75820

Source clickhouse: updated tests

ba94e0f

Source clickhouse: format

7a193f9

Source clickhouse: format

827d059

grishick reviewed Sep 23, 2022

View reviewed changes

VitaliiMaltsev reviewed Sep 23, 2022

View reviewed changes

Source clickhouse: format

2ae34db

Source clickhouse: format

76f828a

sashaNeshcheret requested review from grishick and VitaliiMaltsev September 26, 2022 18:03

grishick approved these changes Sep 26, 2022

View reviewed changes

sashaNeshcheret added 2 commits September 27, 2022 12:10

Merge remote-tracking branch 'origin/master' into omneshcheret/source…

646f505

…-clickhouse-custom-parameters

Source clickhouse: bump version

13295da

github-actions bot added the area/documentation Improvements or additions to documentation label Sep 27, 2022

auto-bump connector version [ci skip]

8956887

sashaNeshcheret merged commit 765bd1b into master Sep 27, 2022

sashaNeshcheret deleted the omneshcheret/source-clickhouse-custom-parameters branch September 27, 2022 11:03

This was referenced Sep 27, 2022

Bump Airbyte version from 0.40.9 to 0.40.10 #17285

Closed

Bump Airbyte version from 0.40.9 to 0.40.10 #17359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source ClickHouse: add custom jdbc params #17031

Source ClickHouse: add custom jdbc params #17031

sashaNeshcheret commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

sashaNeshcheret commented Sep 22, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 22, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

VitaliiMaltsev commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

grishick Sep 23, 2022

sashaNeshcheret Sep 26, 2022 •

edited

Loading

VitaliiMaltsev Sep 23, 2022

VitaliiMaltsev Sep 23, 2022

VitaliiMaltsev Sep 23, 2022

github-actions bot commented Sep 26, 2022

github-actions bot commented Sep 26, 2022

sashaNeshcheret commented Sep 26, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 26, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Sep 27, 2022

sashaNeshcheret commented Sep 27, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 27, 2022 •

edited by github-actions bot

Loading

		import static java.time.temporal.ChronoUnit.SECONDS;
		import static org.junit.jupiter.api.Assertions.*;

Source ClickHouse: add custom jdbc params #17031

Source ClickHouse: add custom jdbc params #17031

Conversation

sashaNeshcheret commented Sep 22, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

github-actions bot commented Sep 22, 2022

sashaNeshcheret commented Sep 22, 2022 • edited by github-actions bot Loading

Build Passed

sashaNeshcheret commented Sep 22, 2022 • edited by github-actions bot Loading

Build Passed

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 22, 2022

VitaliiMaltsev commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

grishick Sep 23, 2022

Choose a reason for hiding this comment

sashaNeshcheret Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

VitaliiMaltsev Sep 23, 2022

Choose a reason for hiding this comment

VitaliiMaltsev Sep 23, 2022

Choose a reason for hiding this comment

VitaliiMaltsev Sep 23, 2022

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2022

github-actions bot commented Sep 26, 2022

sashaNeshcheret commented Sep 26, 2022 • edited by github-actions bot Loading

Build Passed

sashaNeshcheret commented Sep 26, 2022 • edited by github-actions bot Loading

Build Passed

github-actions bot commented Sep 27, 2022

sashaNeshcheret commented Sep 27, 2022 • edited by github-actions bot Loading

sashaNeshcheret commented Sep 27, 2022 • edited by github-actions bot Loading

sashaNeshcheret commented Sep 22, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 22, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret Sep 26, 2022 •

edited

Loading

sashaNeshcheret commented Sep 26, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 26, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 27, 2022 •

edited by github-actions bot

Loading

sashaNeshcheret commented Sep 27, 2022 •

edited by github-actions bot

Loading